feat: per-file tracking for folder uploads in upload queue#3786
feat: per-file tracking for folder uploads in upload queue#3786Crash-- wants to merge 10 commits intofix/upload-queue-fileId-identityfrom
Conversation
Crash--
requested review from
JF-Cozy,
doubleface,
lethemanh,
rezk2ll and
zatteo
as code owners
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughThis PR refactors the upload module's folder and file handling mechanism. It removes custom folder detection from Dropzone in favor of react-dropzone's File object processing. The core changes introduce fileId-based item identification throughout the upload queue (replacing name-based matching), add server-side folder resolution via new utilities ( Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
BundleMonFiles updated (2)
Unchanged files (19)
Total files change +1.72KB +0.03% Groups updated (1)
Unchanged groups (2)
Final result: ✅ View report in BundleMon website ➡️ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/modules/upload/Dropzone.jsx (1)
51-53: Minor redundancy:filesToUploadalias is unnecessary.Since the conditional folder-handling logic was removed,
filesToUploadis now just a direct alias forfiles. Consider usingfilesdirectly in theuploadFilescall.♻️ Suggested simplification
- // react-dropzone sets file.path on each File with the relative path. - // addToUploadQueue uses file.path to detect and handle folder uploads. - const filesToUpload = files dispatch( uploadFiles( - filesToUpload, + files, // react-dropzone sets file.path for folder uploads displayedFolder.id,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/upload/Dropzone.jsx` around lines 51 - 53, Remove the unnecessary alias by deleting the declaration const filesToUpload = files and pass files directly to uploadFiles (and any other call sites that currently use filesToUpload) in Dropzone.jsx; ensure there are no remaining references to filesToUpload (e.g., within addToUploadQueue or uploadFiles invocation) so the code uses the original files variable directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/modules/upload/index.js`:
- Around line 157-163: The RESOLVE_FOLDER_ITEMS reducer currently only updates
existing queue items (using state.map) so files discovered by flattenEntries
(action.resolvedItems) for folder drops are never added; change the
RESOLVE_FOLDER_ITEMS case to merge resolved items into the state by: for each
item in action.resolvedItems, update matching state entries (by fileId) and
collect any resolved items whose fileId is not present in state and append them
(or alternatively dispatch ADD_TO_UPLOAD_QUEUE with those new items); reference
the reducer case handling RESOLVE_FOLDER_ITEMS and the payload field
action.resolvedItems, and ensure this complements where preliminaryItems and
flattenEntries produce folder-derived items.
---
Nitpick comments:
In `@src/modules/upload/Dropzone.jsx`:
- Around line 51-53: Remove the unnecessary alias by deleting the declaration
const filesToUpload = files and pass files directly to uploadFiles (and any
other call sites that currently use filesToUpload) in Dropzone.jsx; ensure there
are no remaining references to filesToUpload (e.g., within addToUploadQueue or
uploadFiles invocation) so the code uses the original files variable directly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 417b7482-d5ab-41f5-a3e1-5bbc0b39a228
📒 Files selected for processing (5)
src/modules/upload/Dropzone.jsxsrc/modules/upload/DropzoneDnD.jsxsrc/modules/upload/UploadQueue.jsxsrc/modules/upload/index.jssrc/modules/upload/index.spec.js
Crash--
force-pushed
the
feat/folder-upload-per-file-tracking
branch
from
a400368 to
a988eef
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Crash--
force-pushed
the
feat/folder-upload-per-file-tracking
branch
from
a988eef to
bbfe791
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Crash--
force-pushed
the
feat/folder-upload-per-file-tracking
branch
from
bbfe791 to
702fcf9
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Enregistrement.de.l.ecran.2026-04-06.a.17.09.45.mov |
There was a problem hiding this comment.
Pull request overview
This PR improves folder uploads by representing each file as its own upload-queue item (using relative paths as identifiers), enabling per-file progress/error handling, and creating the corresponding server-side folder structure before uploads start.
Changes:
- Add folder-resolution flow (
flattenEntries*+RESOLVE_FOLDER_ITEMS) to expand folder drops into per-file queue items withrelativePathandfolderId. - Update upload processing to upload each item into its resolved target directory (
folderId) and to key reducer updates byfileId. - Adjust Dropzone behavior to rely on
file.path(react-dropzone) and add tests covering the new flattening logic and reducer merge behavior.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/modules/upload/UploadQueue.jsx | Displays relative paths in the UI by deriving a display-friendly file.name from relativePath. |
| src/modules/upload/index.js | Introduces folder resolution + queue merge (RESOLVE_FOLDER_ITEMS), updates upload execution to target per-item folderId, and adds 409 handling in createFolder. |
| src/modules/upload/index.spec.js | Adds unit tests for queue merging and both flattening strategies (FileSystemEntry-based and file.path-based). |
| src/modules/upload/DropzoneDnD.jsx | Documents the synchronous nature of react-dnd drops (so webkitGetAsEntry() remains valid). |
| src/modules/upload/Dropzone.jsx | Simplifies drop handling to rely on file.path for folder detection/handling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const mapStateToProps = state => { | ||
| const rawQueue = getUploadQueue(state) | ||
|
|
||
| // Replace file.name with relativePath for display when available | ||
| const queue = rawQueue.map(item => { | ||
| if (!item.relativePath) return item | ||
| return { | ||
| ...item, | ||
| file: { | ||
| name: item.relativePath, | ||
| type: item.file?.type, | ||
| size: item.file?.size | ||
| } | ||
| } | ||
| }) |
There was a problem hiding this comment.
mapStateToProps always creates a new queue array via rawQueue.map(...) even when no items have relativePath. This breaks referential equality and can trigger re-renders of the connected UploadQueue on any store update. Consider memoizing this transformation (e.g., reselect) or only mapping when at least one item needs a display override.
There was a problem hiding this comment.
@copilot apply changes based on this feedback
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Crash--
force-pushed
the
fix/upload-queue-fileId-identity
branch
from
20a8482 to
fa56779
Compare
…dead code - Reducer falls back to matching on file.name when fileId is absent, ensuring Flagship upload path (which dispatches without fileId) works. - extractFilesEntries now calls webkitGetAsEntry() only once per item. - Remove dead code: removeFileToUploadQueue (unused export). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When a folder is dropped, each file now appears as its own item in the upload queue with its relative path (e.g. "photos/2024/img.jpg"), individual progress tracking, and per-file error handling. - Add flattenEntriesFromPaths: uses file.path from react-dropzone to detect folder structure, create server-side folders, and produce flat queue items with relativePath and folderId - Add flattenEntries: FileSystemEntry-based variant for DropzoneDnD where webkitGetAsEntry() is still valid (synchronous drop handler) - Refactor processNextFile: remove isDirectory branch, use item.folderId - Fix createFolder: handle 409 conflict (folder already exists) - Simplify Dropzone.jsx: always pass files (react-dropzone sets file.path) - UploadQueue.jsx: display relativePath instead of file.name in the queue - Remove dead code: uploadDirectory, canHandleFolders in Dropzone.jsx Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The upload queue now appears instantly when files are dropped. Server- side folder creation happens in the background, then queue items are updated with resolved folderId/relativePath via RESOLVE_FOLDER_ITEMS. Previously the queue only appeared after all folders were created, causing a noticeable delay on slow connections. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Extract helpers to reduce cyclomatic complexity flagged by CodeScene: - buildPreliminaryItems: builds initial queue items for immediate display - resolveServerFolders: dispatches to the right flatten strategy - cleanFilePath / makeFlatItem / makeFolderItem: reduce branching in flattenEntriesFromPaths Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
When dropping a folder via DropzoneDnD, directory entries have file=null so buildPreliminaryItems filters them out. flattenEntries then discovers the actual files inside. The RESOLVE_FOLDER_ITEMS reducer now appends items whose fileId doesn't exist in the queue yet, instead of only updating existing ones. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…) merge Agent-Logs-Url: https://github.com/linagora/twake-drive/sessions/0b905f1c-c853-4a77-9954-fb0f711b7c9d Co-authored-by: Crash-- <1107936+Crash--@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Remove unused performUpload helper after rebase. Fix Prettier formatting on ternary expressions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Crash--
force-pushed
the
feat/folder-upload-per-file-tracking
branch
from
b37e938 to
f2132dd
Compare
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Extract updateQueueItem, uploadSingleFile, hasFolderEntries, notifyFolderError, and ensureCallback to reduce cyclomatic complexity and method size. Code Health: 7.7 → 9.01. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
(1 files improve in Code Health)
Gates Passed
See analysis details in CodeScene
View Improvements
| File | Code Health Impact | Categories Improved |
|---|---|---|
| index.js | 8.79 → 9.01 | Complex Method, Excess Number of Function Arguments |
Quality Gate Profile: The Bare Minimum
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
|
|
||
| const notifyFolderError = () => { | ||
| if (typeof window !== 'undefined' && typeof window.alert === 'function') { | ||
| window.alert('The folder upload could not be prepared. Please try again.') |
There was a problem hiding this comment.
let's use showAlert like everywhere else
| ) => { | ||
| const targetDirId = folderId ?? dirID | ||
| const encryptionKey = flag('drive.enable-encryption') | ||
| ? await getEncryptionKeyFromDirId(client, targetDirId) |
There was a problem hiding this comment.
This might need explicit validation since subfolders get their own key lookup. If keys don't inherit to newly created subfolders, encrypted uploads break silently.
There was a problem hiding this comment.
But in this case, it means that we already have the issue, right? I didn't change the current behavior.
| return { | ||
| ...item, | ||
| file: { | ||
| name: item.relativePath, |
There was a problem hiding this comment.
This might break icon resolution since we pass a path instead of a file name
There was a problem hiding this comment.
Hmm interesting. I'll check that and add a test for that.
Summary
photos/2024/img.jpg), individual progress tracking, and per-file error handlingfileIdinstead offile.name)file.namewhenfileIdis absent (Flagship compatibility)extractFilesEntriescallswebkitGetAsEntry()only once per itemcreateFolderhandles 409 conflict (folder already exists)file.pathfor react-dropzone,FileSystemEntryfor react-dnd (DropzoneDnD)Known limitations
Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes
Tests